pkg/planner/core/planbuilder.go (1)

2479-2519: 📐 Maintainability & Code Quality | 🔵 Trivial

Minor inconsistency: MVIndex/columnar indexes not skipped like in getMustAnalyzedColumns.

getMustAnalyzedColumns (same file, lines 2268-2295) explicitly skips idx.MVIndex || idx.IsColumnarIndex() when picking indexed columns, but this loop does not. Harmless today (the resulting IDs simply won't match anything in colsInfo), but worth aligning for consistency/future-proofing.
[optional_refactor_low_reward]

♻️ Optional consistency fix

 		for _, idx := range tblInfo.Indices {
-			if idx.State != model.StatePublic || len(idx.Columns) == 0 {
+			if idx.State != model.StatePublic || len(idx.Columns) == 0 || idx.MVIndex || idx.IsColumnarIndex() {
 				continue
 			}
 			fullStatsCols[tblInfo.Columns[idx.Columns[0].Offset].ID] = struct{}{}
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/planner/core/planbuilder.go` around lines 2479 - 2519, Align
getFullStatsColsAndRatio with getMustAnalyzedColumns by skipping MVIndex and
columnar indexes when collecting index-derived column IDs. Update the fallback
loop over tblInfo.Indices in PlanBuilder.getFullStatsColsAndRatio so it only
adds the first column for public, non-MV, non-columnar indexes, keeping behavior
consistent and future-proof. Use the existing idx.MVIndex and
idx.IsColumnarIndex checks already used elsewhere in planbuilder.go.

pkg/executor/test/analyzetest/options/analyze_saved_options_test.go (1)

40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing revert for tidb_analyze_non_predicate_column_ratio global var pin (inconsistent with sibling vars in the same function).

Both TestSavedAnalyzeOptions and TestSavedPartitionAnalyzeOptions snapshot and defer-restore tidb_persist_analyze_options (and TestSavedAnalyzeOptions also does so for tidb_auto_analyze_ratio), but the newly added tidb_analyze_non_predicate_column_ratio = 1 line right next to them is never reverted. Since this is a process-wide atomic global holder (per the PR's variable-definition layer), leaving it at 1 can leak into subsequently-run tests in the same package binary.

♻️ Suggested pattern (apply per occurrence)

-	tk.MustExec("set global tidb_analyze_non_predicate_column_ratio = 1")
+	originalRatio := tk.MustQuery("select @@global.tidb_analyze_non_predicate_column_ratio").Rows()[0][0].(string)
+	defer func() {
+		tk.MustExec(fmt.Sprintf("set global tidb_analyze_non_predicate_column_ratio = %v", originalRatio))
+	}()
+	tk.MustExec("set global tidb_analyze_non_predicate_column_ratio = 1")

As per path instructions for **/*_test.go: "Test files: ... keep test changes minimal and deterministic."

Also applies to: 142-144

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/executor/test/analyzetest/options/analyze_saved_options_test.go` around
lines 40 - 42, The test setup in TestSavedAnalyzeOptions and the related
saved-options test pins tidb_analyze_non_predicate_column_ratio globally but
never restores it, which can leak state into later tests. Update the test logic
around the existing snapshot/defer pattern used for tidb_persist_analyze_options
and tidb_auto_analyze_ratio so the new global variable is also saved before the
override and restored afterward, keeping the behavior deterministic across the
package binary.

Source: Path instructions

pkg/executor/test/analyzetest/columns/analyze_columns_with_test.go (1)

38-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing revert for tidb_analyze_non_predicate_column_ratio global var pin.

Each of these six spots sets global tidb_analyze_non_predicate_column_ratio = 1 but never restores the previous value, unlike other global sysvars mutated in the same test bodies elsewhere in this file family (e.g. tidb_persist_analyze_options, tidb_auto_analyze_ratio in analyze_saved_options_test.go, which snapshot the original value and defer a reset). Per the PR context, this variable is backed by a process-wide atomic global holder, not scoped to the per-test mock store/domain, so a leaked value of 1 can silently persist into later tests in the same package binary, masking or skewing the intended default (0.1) reduction behavior for any test that doesn't explicitly re-pin the ratio (the new TestAnalyzeNonPredicateColumnRatio itself correctly reverts via defer, showing the intended convention).

Since none of these six tests actually need a specific ratio (they exist purely to keep pre-existing behavior stable), consider following the same snapshot+defer pattern.

♻️ Suggested pattern (apply per occurrence)

-			tk.MustExec("set global tidb_analyze_non_predicate_column_ratio = 1")
+			originalRatio := tk.MustQuery("select @@global.tidb_analyze_non_predicate_column_ratio").Rows()[0][0].(string)
+			defer func() {
+				tk.MustExec(fmt.Sprintf("set global tidb_analyze_non_predicate_column_ratio = %v", originalRatio))
+			}()
+			tk.MustExec("set global tidb_analyze_non_predicate_column_ratio = 1")

As per path instructions for **/*_test.go: "Test files: ... keep test changes minimal and deterministic."

Also applies to: 109-111, 189-191, 269-271, 398-400, 512-514

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/executor/test/analyzetest/columns/analyze_columns_with_test.go` around
lines 38 - 40, The `TestAnalyze...` cases in `analyze_columns_with_test.go` pin
`tidb_analyze_non_predicate_column_ratio` with `set global` but never restore
it, which can leak the process-wide value into later tests. Update each
occurrence to snapshot the current value before calling the relevant
`tk.MustExec` setup, then `defer` a reset back to the original setting after the
test body, matching the existing pattern used for other global sysvars in this
test family. Use the `tidb_analyze_non_predicate_column_ratio` setup sites in
the affected test functions as the anchor points for the fix.

Source: Path instructions

pkg/executor/test/analyzetest/analyze_test.go (1)

639-641: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing revert for tidb_analyze_non_predicate_column_ratio global var pin.

Same concern as in analyze_columns_with_test.go: these three spots set the global ratio to 1 without capturing/restoring the original value, whereas sibling global vars in these same functions (e.g. tidb_persist_analyze_options) do use snapshot+defer reset. Given this variable is backed by a process-wide atomic global holder, a leaked value can affect later tests in the same package binary.

♻️ Suggested pattern (apply per occurrence)

-			tk.MustExec("set global tidb_analyze_non_predicate_column_ratio = 1")
+			originalRatio := tk.MustQuery("select @@global.tidb_analyze_non_predicate_column_ratio").Rows()[0][0].(string)
+			defer func() {
+				tk.MustExec(fmt.Sprintf("set global tidb_analyze_non_predicate_column_ratio = %v", originalRatio))
+			}()
+			tk.MustExec("set global tidb_analyze_non_predicate_column_ratio = 1")

As per path instructions for **/*_test.go: "Test files: ... keep test changes minimal and deterministic."

Also applies to: 1067-1069, 1164-1166

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/executor/test/analyzetest/analyze_test.go` around lines 639 - 641, The
test is pinning tidb_analyze_non_predicate_column_ratio globally without
restoring it, which can leak state into later package tests. In each affected
test in analyze_test.go, snapshot the current value before the set in the
relevant test function, use the existing tk.MustExec call to set it to 1, and
add a defer to restore the original value just like the nearby
tidb_persist_analyze_options handling. Refer to the affected test functions
containing the tidb_analyze_non_predicate_column_ratio setup and apply the same
pattern at each occurrence.

Source: Path instructions